fix 1677351. Restrict CLO to single namespace and allow Jaeger to reuse EO without installing CL #85

jcantrill · 2019-02-20T16:28:59Z

This PR fixes https://bugzilla.redhat.com/show_bug.cgi?id=1677351 by:

Restricting CLO to install/watch of targeted namespaces
Restricting EO to install/watch globally for re-use by Jaeger
Breaks out EO into separate CSV

openshift-ci-robot · 2019-02-20T16:29:14Z

Hi @jcantrill. Thanks for your PR.

I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jcantrill · 2019-02-20T16:31:50Z

Any reason we need to wait until post Feb 22 to merge this? Functionally it should be the same aside from requiring users to explicitly install EO and CLO

cc @anpingli

ewolinetz · 2019-02-20T20:08:35Z

looks correct to me based on our earlier conversation

SamiSousa · 2019-02-20T20:17:15Z

Does CLO require EO to work? Is there a requirement that EO be installed before CLO now that they are separated?

ewolinetz · 2019-02-20T20:31:34Z

@SamiSousa EO requires either CLO or the Jaeger operator so that required secrets for the resulting deployments created by EO are available, as well as being responsible for creating the CR that the EO watches.

We want EO installed before so that we do not run into the dependency resolving issue where CLO is restricted to a single namespace and then EO is installed in the same way (into a single namespace).

SamiSousa · 2019-02-21T22:39:56Z

...ty-operators/elasticsearch-operator/elasticsearch-operator.v0.0.1.clusterserviceversion.yaml

+
+    Once installed, the operator provides the following features:
+    * **Create/Destroy**: Deploy an Elasticsearch cluster to the same namespace in which the Elasticsearch custom resource is created.
+


Are there any docs for using this operator with CLO or jaeger? Could these be included either in this description or the description of CLO/jaeger?

I'm not sure what additional documentation applies other then:

This operator only supports OKD Cluster Logging and Jaeger. It is tightly coupled to each and is not currently capable of being used as a general purpose manager of Elasticsearch clusters running on OKD.

What more is there to say? The operator will act upon any elasticsearch that is created. It is immaterially that CLO or Jaeger created the resource.

That verbiage in the description is sufficient. I thought there might have been a more complicated setup, but as long as it's clear in the EO description that it is meant for CLO and jaeger, and in the CLO description that EO is required for some functionality, then it's fine.

...ty-operators/elasticsearch-operator/elasticsearch-operator.v0.0.1.clusterserviceversion.yaml

community-operators/cluster-logging/clusterlogging.v0.0.1.clusterserviceversion.yaml

...ty-operators/elasticsearch-operator/elasticsearch-operator.v0.0.1.clusterserviceversion.yaml

SamiSousa · 2019-02-27T20:02:32Z

community-operators/cluster-logging/cluster-logging.v0.0.1.clusterserviceversion.yaml

+
    Once installed, the Cluster Logging Operator provides the following features:
-    * **Create/Destroy**: Launch and create an aggregated logging stack in the `openshift-logging` namespace.
+    * **Create/Destroy**: Launch and create an aggregated logging stack to support the entire OKD cluster.


If the operator only supports SingleNamespace, how is it supposed to support "the entire OKD cluster"? Unless the application pod is responsible for that.

The logging stack supports the entire OKD cluster. The cluster-logging-operator only runs in a single namespace.

richm · 2019-03-04T03:11:16Z

What's the holdup here?

…se EO without installing CL bug 1684329. Include openshift-logging in CSV example bug 1683701. CLO ability to list ClusterLogging

SamiSousa

/lgtm

SamiSousa · 2019-03-05T03:45:11Z

Operator bundles for cluster-logging and elasticsearch-operator have been pushed into the community-operators namespace on quay

richm · 2019-03-05T14:33:25Z

What is the process for getting updates to community operators into a 4.0 cluster? How long does it take from when a PR merges to when I can install a 4.0 cluster with the fixes in it?

aravindhp · 2019-03-05T15:36:49Z

We sync with Quay every hour, so you should see updates in that time frame. Please ensure your cluster is running with operator-framework/operator-marketplace#125 which fixes a bug with updates.

richm · 2019-03-05T15:45:35Z

Please ensure your cluster is running with operator-framework/operator-marketplace#125

How do I ensure that? If I create a cluster now, will it have that fix?

aravindhp · 2019-03-05T16:49:20Z

If you use the latest installer built from master, it should have that fix. You can also confirm by executing oc adm release info `oc get clusterversion -o yaml | grep image | head -n 1 | awk '{print $2}'` --commits | grep marketplace and checking the commit hash against https://github.com/operator-framework/operator-marketplace/. Please note that you need a 4.0 oc binary for the command to work.

openshift-ci-robot requested review from SamiSousa and aravindhp February 20, 2019 16:29

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 20, 2019

openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 20, 2019

jcantrill force-pushed the 1677351_part_deux branch from a1c5a3c to 42eda35 Compare February 21, 2019 17:23

SamiSousa reviewed Feb 21, 2019

View reviewed changes

SamiSousa suggested changes Feb 21, 2019

View reviewed changes

...ty-operators/elasticsearch-operator/elasticsearch-operator.v0.0.1.clusterserviceversion.yaml Outdated Show resolved Hide resolved

community-operators/cluster-logging/clusterlogging.v0.0.1.clusterserviceversion.yaml Show resolved Hide resolved

jcantrill force-pushed the 1677351_part_deux branch from 42eda35 to d2307e0 Compare February 26, 2019 19:15

SamiSousa suggested changes Feb 26, 2019

View reviewed changes

...ty-operators/elasticsearch-operator/elasticsearch-operator.v0.0.1.clusterserviceversion.yaml Outdated Show resolved Hide resolved

jcantrill force-pushed the 1677351_part_deux branch 2 times, most recently from e412f19 to 6ec8184 Compare February 27, 2019 15:13

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2019

jcantrill force-pushed the 1677351_part_deux branch 2 times, most recently from d9934d4 to 49b8812 Compare February 27, 2019 19:52

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2019

jcantrill force-pushed the 1677351_part_deux branch 2 times, most recently from 83c427e to d936566 Compare February 27, 2019 20:21

SamiSousa suggested changes Feb 27, 2019

View reviewed changes

jcantrill force-pushed the 1677351_part_deux branch from d936566 to e7edf69 Compare March 1, 2019 14:31

jcantrill force-pushed the 1677351_part_deux branch from e7edf69 to 292fa9a Compare March 4, 2019 19:41

bug 1677351. Restrict CLO to single namespace and allow Jaeger to reu…

38f204a

…se EO without installing CL bug 1684329. Include openshift-logging in CSV example bug 1683701. CLO ability to list ClusterLogging

jcantrill force-pushed the 1677351_part_deux branch from 292fa9a to 38f204a Compare March 4, 2019 21:27

openshift-ci-robot assigned SamiSousa Mar 5, 2019

SamiSousa approved these changes Mar 5, 2019

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 5, 2019

SamiSousa merged commit e18c827 into operator-framework:master Mar 5, 2019

jcantrill deleted the 1677351_part_deux branch March 5, 2019 13:45

jcantrill mentioned this pull request Mar 5, 2019

install logging in openshift-logging namespace only #160

Closed


		Once installed, the operator provides the following features:
		* Create/Destroy: Deploy an Elasticsearch cluster to the same namespace in which the Elasticsearch custom resource is created.

fix 1677351. Restrict CLO to single namespace and allow Jaeger to reuse EO without installing CL #85

fix 1677351. Restrict CLO to single namespace and allow Jaeger to reuse EO without installing CL #85

Uh oh!

Conversation

jcantrill commented Feb 20, 2019

Uh oh!

openshift-ci-robot commented Feb 20, 2019

Uh oh!

jcantrill commented Feb 20, 2019

Uh oh!

ewolinetz commented Feb 20, 2019

Uh oh!

SamiSousa commented Feb 20, 2019

Uh oh!

ewolinetz commented Feb 20, 2019

Uh oh!

SamiSousa Feb 21, 2019

Choose a reason for hiding this comment

Uh oh!

jcantrill Feb 26, 2019

Choose a reason for hiding this comment

Uh oh!

SamiSousa Feb 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SamiSousa Feb 27, 2019

Choose a reason for hiding this comment

Uh oh!

richm Mar 5, 2019

Choose a reason for hiding this comment

Uh oh!

richm commented Mar 4, 2019

Uh oh!

SamiSousa left a comment

Choose a reason for hiding this comment

Uh oh!

SamiSousa commented Mar 5, 2019

Uh oh!

richm commented Mar 5, 2019

Uh oh!

aravindhp commented Mar 5, 2019

Uh oh!

richm commented Mar 5, 2019

Uh oh!

aravindhp commented Mar 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

SamiSousa Feb 27, 2019 •

edited

Loading